-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(i): Sort out invalid testing framework node indexing #3068
fix(i): Sort out invalid testing framework node indexing #3068
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3068 +/- ##
===========================================
+ Coverage 79.35% 79.36% +0.01%
===========================================
Files 345 345
Lines 26557 26557
===========================================
+ Hits 21074 21076 +2
+ Misses 3956 3955 -1
+ Partials 1527 1526 -1
Flags with carried forward coverage won't be shown. Click here to find out more. see 6 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
7b56f43
to
0e777fd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks for documenting everything so well. I'm also in favor of removing the getNodes
utilities.
and mark a todo
0e777fd
to
6b9be21
Compare
Relevant issue(s)
Resolves #3065
Description
The main bug was only visible on sourcehub acp using http, due to the identity being copied with the audience value of another node's host failing authentication (the bearer tokens should be unique using correct node's audience). The biggest issue was the way we use
getNodes
andgetNodeCollections
. I would be in favor of completely removing them as they are more troublesome than the utility they provide.Future:
getNodeCollections
(Clean up test utils, make helpers to avoid code duplication and remove confusing util functions #3069)getNodes
(Clean up test utils, make helpers to avoid code duplication and remove confusing util functions #3069)How has this been tested?
getNodes
andgetNodeCollections